Skip to content

Fix typo increase counter on connection close instead of decreasing it#1494

Merged
jfallows merged 2 commits into
aklivity:developfrom
akrambek:bug/tcp-counter
Jun 12, 2025
Merged

Fix typo increase counter on connection close instead of decreasing it#1494
jfallows merged 2 commits into
aklivity:developfrom
akrambek:bug/tcp-counter

Conversation

@akrambek
Copy link
Copy Markdown
Contributor

No description provided.

Comment on lines +382 to +383
ServerSocketChannel srv = (ServerSocketChannel) key.channel();
SocketChannel client = srv.accept();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ServerSocketChannel srv = (ServerSocketChannel) key.channel();
SocketChannel client = srv.accept();
SocketChannel client = server.accept();

Comment on lines +391 to +394
for (int i = 1; i < 3; i++)
{
k3po.notifyBarrier("CONNECTION_ACCEPTED_" + i);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (int i = 1; i < 3; i++)
{
k3po.notifyBarrier("CONNECTION_ACCEPTED_" + i);
}
k3po.notifyBarrier("CONNECTION_ACCEPTED_1");
k3po.notifyBarrier("CONNECTION_ACCEPTED_2");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be accepting 1 connection then notifying that 2 connections have been accepted, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are right

}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of accepted should equal 2 here?
Let's add an assertion to document that test requirement.

@jfallows jfallows merged commit 941ebec into aklivity:develop Jun 12, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants